Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't write to parent repo #1524

Closed

Conversation

alexlarsson
Copy link
Member

In _try_clone_from_payload_link, don't try to do the clone in the
parent repo, because we don't want to modify that. parent repos are
typically used when you want a shared, immutable base.

For example in flatpak, the parent repo is the system repo which you
don't have write access to, so any modification to it will fail with
EACCES, making it impossible to install via the system helper.

In _try_clone_from_payload_link, don't try to do the clone in the
parent repo, because we don't want to modify that. parent repos are
typically used when you want a shared, immutable base.

For example in flatpak, the parent repo is the system repo which you
don't have write access to, so any modification to it will fail with
EACCES, making it impossible to install via the system helper.
@giuseppe
Copy link
Member

do you have a log with the error? If possible, I'd like to fix it in a way that the lookup is still done in the parent repository, but no write should happen there.

@alexlarsson
Copy link
Member Author

What fails is the glnx_open_tmpfile_linkable_at()->open(O_TMPFS) gets EACCES in _check_support_reflink() on the parent repo, because its is only writable by root.

But also, even if this would not break, the entire thing seems to depend on any reflinks to work if _check_support_reflink returns true. However if the parent and child repos are on different filesystem you might not be able to reflink between them, even if both are on reflink supporting filesystems.

@cgwalters
Copy link
Member

@rh-atomic-bot r+ ad11aa1

@rh-atomic-bot
Copy link

⌛ Testing commit ad11aa1 with merge 28c7bc6...

@cgwalters
Copy link
Member

Unfortunately while we do run the flatpak unit tests as part of our CI, that doesn't cover "real" usage like this. And I personally tend to mostly only test ostree git with rpm-ostree.

We can clearly wire up more "real" usage testing in CI, it's one of the things I'm working on now. Short term I'll add "flatpak sanity checks" to my "before ostree release" checklist.

@rh-atomic-bot
Copy link

☀️ Test successful - status-atomicjenkins
Approved by: cgwalters
Pushing 28c7bc6 to master...

@alexlarsson
Copy link
Member Author

We should probably backport this fix to F28...

@cgwalters
Copy link
Member

Sure, I can do that, or feel free to do so if you prefer. I'm not immediately reproducing the problem though; does it only occur when the host fs supports reflinks (are you still on btrfs as /)?

@cgwalters
Copy link
Member

cgwalters commented Mar 29, 2018

does it only occur when the host fs supports reflinks

Answer: yes.

@cgwalters
Copy link
Member

(Also just leaving this breadcrumb for future self and others: be sure to pkill -f /usr/libexec/flatpak-system-helper to be sure you're actually testing the new libostree version you installed)

LorbusChris pushed a commit to LorbusChris/ostree-spec that referenced this pull request Oct 23, 2018
This fixes installing via flatpak system helper on systems where the
host fs supports reflinks.

ostreedev/ostree#1524
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants